-
-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Import - start to use civicrm_user_job to track user imports #23245
Conversation
(Standard links)
|
35a76e5
to
411e220
Compare
411e220
to
c0f5de4
Compare
They don't since that would be a security issue - code could default it to upload |
@demeritcowboy ok thanks - good to know |
149f88e
to
9d284cd
Compare
civibot, test this please |
9d57ab5
to
6ce886e
Compare
This adds 1) an acl so only the creator can access and 2) permits anyone with access CiviCRM to GET their own jobs 3) permission on CREATE is set to administer civicrm I am thinking that create might be too strict but it might be better to start this way. Likewise we probably want sysadmins to be able to access other people's jobs but unless we have a plan now for what permissions we want we can punt the question by keeping these strict for now
6ce886e
to
7b057b6
Compare
#23260 extends this PR to make some improvements to the MapField form & template & Preview - it might be easier to try all together - all focussed on Contact import flow |
test this please |
Console log is unclear ... I found INFO: Processing JUnit |
This is only fails on an infra related issue - but might as well get a clean slate - test this please |
1 similar comment
This is only fails on an infra related issue - but might as well get a clean slate - test this please |
unrelated test fails - however, the tests have gotten messed up now on the ones that build on this during the test issues over the last bit. @colemanw & I had previous discussed & @colemanw blessed the PR that builds off the PR that builds off this PR to merge. However, now that one has a problem - so I'm going to merge this based on the authorization of the follow up so I can start to unravel what is going on. Note that the mitigating factor in all this is that more PRs are coming with more r-run to do on them over the same bit of code. |
Overview
Import - start to use civicrm_user_job to track user imports.
While this is primarily a code change to support further cleanups & addition of functionality it does fix one issue - the loss of the submitted values in the data source section of the form when going forwards and back - with this PR the following works
Note that this also works for csv with 'skipColumnHeader' - annoyingly it doesn't for the file upload field - the defaults are set but I don't quite know how defaults work with file upload fields - maybe @colemanw or @demeritcowboy does?
Before
civicrm_user_job has been added to the schema but we have not started to use it
After
When submitting the first form (DataSource) in the Import contact flow a record is added to the civicrm_user_job table holding
submitted_values
- these will be added to by those submitted on other forms in the flow - notably the mapping - so the job could be picked up & run later & also so we don't have to do a weird dance to pass them arounDataSource
- values relating to the datasource - so fartable_name
- the created table andcolumn_headers
- the relevant headersTechnical Details
Next steps include updating the parser class to use the userJobID to get the data it needs rather than pass in a huge array on 'run' - ideally it would take
$userJobID
in theconstruct()
and should not accept any other values. Lots more cleanup to comeComments
Merging #23253 will improve readability